Refactoring of StdoutProxy to take app_session from the caller #1854
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes makes StdoutProxy to take the exact app_session from by taking it as optional argument in the constructor instead finding it by calling get_app_session() globally.
It can avoid confliction of printing output within the patch_stdout context in the ssh session when the multiple ssh connections are performing concurrently.
The changed StdoutProxy now passes the correct app instance from the given app_session in the constructor to the run_in_terminal() in it instead of calling get_app_or_none() globally that can give wrong app instance from the prompt session in the last ssh connection.
The key usage has been added into the example ssh/asyncssh-server.py.
How to test
Result
Background & motivation
I found the issue when I used patch_stdout in the asyncssh-server.py based custom ssh server.
Basically the print within the patch_stdout block was messed because of the multiple the patch_stdout context interfere the other one from the other ssh session.
I used "with StdoutProxy(...) as output" in the example code asyncssh-server.py to test this patch. It can more explicitly use the right output object than using "print" within "patch_stdout" block in case of the ssh server example.
I wanted to touch "patch_stdout" to take the app_session too to achieve the goal, but It seems to be dangerous for the existing echo system of this project. so, I tried to modify only the StdoutProxy class to take the additional optional parameter 'app_session'.
Thanks in advance for the feedback / review. and thanks for the good library!